Skip to content

Testing infra: flushPointerMoves seam + TestEditor polish + clipboard DI#44

Merged
kostyafarber merged 16 commits into
mainfrom
firmclaw/testing-infra
Apr 22, 2026
Merged

Testing infra: flushPointerMoves seam + TestEditor polish + clipboard DI#44
kostyafarber merged 16 commits into
mainfrom
firmclaw/testing-infra

Conversation

@kostyafarber
Copy link
Copy Markdown
Collaborator

Re-submits the work from #43 against `main` — that PR merged into its intermediate base (`firmclaw/domain-model-sweep`) which landed on main before the stack caught up, so the testing-infra commits never made it to main. Same commits, clean diff against main now.

Summary

  • `ToolManager.flushPointerMoves()` public seam — drains the rAF-coalesced pointer-move queue synchronously; kills 4× `vi.stubGlobal("requestAnimationFrame", ...)` blocks
  • `TestEditor.pointerMove` calls the seam internally
  • `Hand.outcome.test.ts` → `Hand.test.ts` via TestEditor (asserts on `editor.pan`)
  • `Shape.outcome.test.ts` → `Shape.test.ts` via TestEditor (asserts on `glyph.contours`)
  • `NativeBridge.test.ts` — 5 session-lifecycle tests against real Rust engine
  • `Clipboard.test.ts` — 6 tests via injected `SystemClipboard` (VSCode-style DI)
  • `Clipboard` class now takes a `SystemClipboard` interface through `ClipboardDeps.clipboard`; production wires `electronSystemClipboard` through the Editor constructor; tests inject `InMemorySystemClipboard`
  • Mock-call-count sweep: every `vi.spyOn + mock.calls.length` assertion removed; all remaining tests assert on observable state

Tickets moved

Test plan

  • `pnpm test` green (537 tests)
  • `pnpm lint:check` green
  • `pnpm typecheck` green
  • Pen tool cursor remains responsive on 50K-point glyphs

Called on every pointer move from the pen cursor computed. Iterates
contour points inline (no points() generator allocation) and uses
squared-distance comparison (no Math.sqrt).
Public method that drains any pending pointer-move immediately, bypassing
rAF. Cancels the scheduled frame to avoid a double flush.

Migrates the four rAF-stub blocks in ToolManager.test.ts to use the seam.
Adds a targeted test locking in the contract: flush is synchronous and
cancels the pending frame.

Enables TestEditor.pointerMove to drive the full tool pipeline (gesture →
tool event → state signal → cursor effect → hover update) without
callers needing to stub requestAnimationFrame.
Calls toolManager.flushPointerMoves() after dispatching so the full
pipeline (gesture → tool event → state signal → cursor effect → hover
update) completes before pointerMove returns. Callers no longer need to
stub requestAnimationFrame to observe the effect of a move.

Adds a TestEditor.test.ts with a single contract assertion: two sequential
pointerMove calls both register distinct mouse positions — an rAF-coalesced
implementation would only retain the latest.
Drops hand-built ToolEvent factories (makeDragStart/Drag/DragEnd/DragCancel)
and the transition(state, event) direct-call pattern. New Hand.test.ts
drives the real pointer pipeline through TestEditor and asserts on the
observable side effect: viewport pan.

Three tests cover what users can trigger:
- drag pans the viewport by the screen delta
- escape mid-drag returns to ready and subsequent moves do not pan
- pointer hover in ready state does not pan

Per testing-strategy.md scope item #3.
Drops hand-built ToolEvent factories (makeDragStart/Drag/DragEnd/DragCancel)
and transition(state, event) direct calls. New Shape.test.ts drives the
real pointer pipeline and asserts on the observable side effect: contours
added (or not added) to the glyph.

Three tests cover what users can trigger:
- drag then release commits a closed 4-point rectangle contour
- escape mid-drag discards the preview without committing
- drag smaller than the 3-unit minimum does not commit

Per testing-strategy.md scope item #3.
Covers the runner's priority-resolution contract using stub steps:
- runPointPipeline: no-match passthrough, point-to-point short-circuit
  (wins over closer metrics candidate and prevents later steps from
  running), closest-candidate fallback when no p2p match
- runRotatePipeline: no-match passthrough, first-match semantics

Stubs the step interface directly so the runner is tested in isolation
from the real snap strategies in steps.ts.

Per snapping-tests.md.
Covers the bridge's session contract against the real Rust engine
(shift-node via createBridge):
- no session / null \$glyph at construction
- startEditSession populates \$glyph and reports the glyph name
- endEditSession clears both
- repeat start for the same glyph is a no-op (reference preserved)
- switching glyphs replaces the Glyph instance
- \$glyph notifies subscribers on session start

Doesn't assert on the \$glyph-vs-data-change distinction — that contract
is currently inconsistent between the bridge code and the reactive docs,
and needs its own cleanup before being locked in by tests.

Per bridge-tests.md.
Drives editor.copy/cut/paste through the real command pipeline and
asserts on the resulting glyph state. Stubs window.electronAPI with an
in-memory buffer so the Electron IPC round-trip works in the Node test
environment.

Four tests:
- copy on empty selection returns false
- copy + paste duplicates the selection (validates offset + PasteCommand path)
- cut removes the selected points from the glyph
- paste with nothing on the clipboard is a no-op

Per clipboard-tests.md scope item 1 (tool test layer).
Sweeps the branch's test files for vi.spyOn / mock.calls.length / counter
assertions — the banned pattern per CLAUDE.md Testing ("Assert on state,
not mock calls") and the cleanup history (5f2f503, fa8a829, bd07e9d).

Changes:

ToolManager.test.ts
- Delete "flushPointerMoves drains..." spy+cancelAnimationFrame test
  (added in this branch, pure mock-count anti-pattern)
- Delete "deduplicates pointer move (no force)" — dedup is an optimization
  with no user-observable consequence; counting handleEvent calls is the
  only way to verify it, which is the pattern we ban
- Delete "processes pointer move when force: true" — same reason
- Rewrite "onActivate callback" and "onReturn callback" to observe a
  closure flag instead of vi.spyOn assertion

SnapPipelineRunner.test.ts
- Drop "applied: string[]" execution tracking from the two priority tests;
  the returned result's source/delta already encodes which step won

NativeBridge.test.ts
- Delete the "$glyph signal notifies subscribers" counter test;
  redundant with the other lifecycle tests that assert $glyph.peek() changes

All remaining tests (40 across 7 files) assert on observable state: tool
state, pan, glyph contours, point count, returned values.
Replaces the direct window.electronAPI.clipboard* calls inside the
Clipboard class with an injected ClipboardAdapter boundary. Production
wires electronClipboardAdapter through the Editor constructor; tests
pass an in-memory fake (done in the next commit).

This matches VSCode's IClipboardService + TestClipboardService pattern
and aligns with the existing "real class + fake boundary" convention
already used for the NativeBridge seam in TestEditor.

Changes:
- Add ClipboardAdapter interface to lib/clipboard/types.ts
- Add electronClipboardAdapter (production impl) in electronAdapter.ts —
  throws loudly if window.electronAPI is missing rather than silently
  dropping ops
- ClipboardDeps.adapter required; Clipboard.#write / #read go through it
- Editor constructor accepts optional clipboardAdapter override
  (defaults to electronClipboardAdapter), mirroring the existing bridge
  override pattern
- Re-export ClipboardAdapter and electronClipboardAdapter from the
  clipboard barrel

No behavior change — existing Clipboard.test.ts still stubs the global
and passes. The next commit drops that stub in favor of DI.
Replaces the stubElectronClipboard + vi.stubGlobal("window", ...) hack
with DI. TestEditor now constructs a FakeClipboardAdapter internally and
passes it to super(), and exposes the in-memory buffer as
editor.clipboardBuffer for direct assertion.

The test file is pure — no vi imports, no afterEach, no stubbing. Each
test drives the editor through its public API and reads observable state.

Also adds a new assertion ("copy writes a shift/glyph-data payload") that
verifies the serialized format without a round-trip, using clipboardBuffer
directly. This replaces the implicit "copy returned true" check with
something load-bearing.

5 tests total.
Two changes in one commit since they interact:

1. Rename
   - ClipboardAdapter → SystemClipboard (the interface)
   - electronClipboardAdapter → electronSystemClipboard (prod impl)
   - ClipboardDeps.adapter → ClipboardDeps.systemClipboard (the field)
   - Editor option .clipboardAdapter → .systemClipboard
   - electronAdapter.ts → electronSystemClipboard.ts

   "Adapter" was a pattern name, not a domain name. "SystemClipboard"
   says what the thing is — the OS-level clipboard — and avoids collision
   with the Clipboard orchestrator class.

2. Drop the default-fallback in Editor's constructor

   Previously: systemClipboard was optional on the Editor constructor,
   defaulting to electronSystemClipboard when not provided. That's a
   test-hatch hanging off production code — the optional param only exists
   for tests. Now systemClipboard is required.

   The one production callsite (store.ts) passes electronSystemClipboard
   explicitly. TestEditor passes its InMemorySystemClipboard. No fallback
   path, no "what happens if this is omitted" ambiguity.
Fourth acceptance box for clipboard-tests.md — verifies that
DEFAULT_PASTE_OFFSET compounds across repeated pastes (the #pasteCount
counter on the Clipboard class). After one copy + two pastes the three
resulting contours are at x-offsets 0 / +20 / +40 from the original.
Asserts on sorted x-extents so the test is independent of the contour
array's insertion order (pasteContours prepends on the Rust side).
@kostyafarber kostyafarber merged commit 02b3897 into main Apr 22, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant